Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support submit sleep in scheduler #6858

Merged
merged 1 commit into from
Jan 19, 2024
Merged

Support submit sleep in scheduler #6858

merged 1 commit into from
Jan 19, 2024

Conversation

berland
Copy link
Contributor

@berland berland commented Dec 28, 2023 •
edited
Loading

Issue
Resolves #6856

Approach
Use a singleton object to be responsible for determining whether Job needs to wait more before it proceeds when doing its __call__.

Pre review checklist

  • Read through the code changes carefully after finishing work
  • Make sure tests pass locally (after every commit!)
  • Prepare changes in small commits for more convenient review (optional)
  • PR title captures the intent of the changes, and is fitting for release notes.
  • Updated documentation
  • Ensured that unit tests are added for all new behavior (See
    Ground Rules),
    and changes to existing code have good test coverage.

Pre merge checklist

  • Added appropriate release note label
  • Commit history is consistent and clean, in line with the contribution guidelines.

@berland berland self-assigned this Dec 28, 2023
@berland berland added the release-notes:skip If there should be no mention of this in release notes label Dec 28, 2023
@berland berland force-pushed the support_submit_sleep branch 3 times, most recently from 54e980e to 8049a74 Compare December 28, 2023 15:35
@berland
Copy link
Contributor Author

berland commented Dec 28, 2023

By adding "SUBMIT_SLEEP" to GENERIC_QUEUE_OPTIONS in queue_config.py this can be tested to work with the local driver on the poly case. Will not add that to the PR as it breaks the legacy JobQueue.

@codecov-commenter
Copy link

codecov-commenter commented Dec 28, 2023 •
edited
Loading

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (7ef6fd6) 84.33% compared to head (57ffe2a) 84.40%.
Report is 2 commits behind head on main.

Files Patch % Lines
src/ert/config/queue_config.py 80.00% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #6858      +/-   ##
==========================================
+ Coverage   84.33%   84.40%   +0.07%     
==========================================
  Files         368      367       -1     
  Lines       21871    21871              
  Branches      911      900      -11     
==========================================
+ Hits        18445    18461      +16     
+ Misses       3132     3116      -16     
  Partials      294      294

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jonathan-eq jonathan-eq self-requested a review December 29, 2023 06:52
jonathan-eq
jonathan-eq requested changes Dec 29, 2023
View reviewed changes
Copy link
Contributor

@jonathan-eq jonathan-eq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, very readable code!

src/ert/config/queue_config.py Show resolved Hide resolved
src/ert/scheduler/scheduler.py Outdated Show resolved Hide resolved
@berland
Copy link
Contributor Author

berland commented Jan 2, 2024

There should be some tests for this togheter with MAX_RUNNING. I don't think combining these feature will currently give expected results.

@berland berland force-pushed the support_submit_sleep branch 3 times, most recently from 8b854cc to 6330ecf Compare January 5, 2024 08:34
@xjules
Copy link
Contributor

xjules commented Jan 12, 2024

It needs rebasing.

@berland
Support submit_sleep in Scheduler
57ffe2a 
Notes:
* The test setup possibly allows flakyness
* Picks SUBMIT_SLEEP from queue system configuration,
  but leans towards a future global setting for SUBMIT_SLEEP.
@berland berland force-pushed the support_submit_sleep branch from fd06f65 to 57ffe2a Compare January 18, 2024 07:06
@berland berland changed the title Support submit sleep Support submit sleep in scheduler Jan 18, 2024
xjules
xjules approved these changes Jan 19, 2024
View reviewed changes
Copy link
Contributor

@xjules xjules left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job @berland ! 🚀

@berland berland enabled auto-merge (squash) January 19, 2024 11:31
@jonathan-eq jonathan-eq self-requested a review January 19, 2024 11:35
jonathan-eq
jonathan-eq approved these changes Jan 19, 2024
View reviewed changes
Copy link
Contributor

@jonathan-eq jonathan-eq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@berland berland merged commit 326c498 into equinor:main Jan 19, 2024
44 checks passed
@berland berland deleted the support_submit_sleep branch September 17, 2024 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonathan-eq jonathan-eq jonathan-eq approved these changes

@xjules xjules xjules approved these changes

Assignees

@berland berland

Labels
release-notes:skip If there should be no mention of this in release notes
Projects
SCOUT
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement SUBMIT_SLEEP for Scheduler
4 participants
@berland @codecov-commenter @xjules @jonathan-eq